Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Freeze event spaces ids #7151

Merged
merged 2 commits into from
Jul 29, 2024
Merged

Freeze event spaces ids #7151

merged 2 commits into from
Jul 29, 2024

Conversation

maximyurchuk
Copy link
Collaborator

@maximyurchuk maximyurchuk commented Jul 26, 2024 •
edited
Loading

Changelog entry

It intended to prevent situations like
f173d0f#diff-34086e3887870dc241014e054279e9e08bba32472b00e4952866315fea4793db
(enums has different values in 24-2 and 24-3)

Real enum values was generated by the following code
#7152

Changelog category

  • Not for changelog (changelog entry is not required)

Additional information

...

@maximyurchuk maximyurchuk requested review from a team as code owners July 26, 2024 16:02
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 26, 2024 •
edited
Loading

⚪ 2024-07-26 16:06:16 UTC Pre-commit check for e12ca69 has started.
⚪ 2024-07-26 16:09:27 UTC Check linux-x86_64-release-asan is running...
🔴 2024-07-26 16:09:40 UTC Build failed. see the logs.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 26, 2024 •
edited
Loading

⚪ 2024-07-26 16:06:32 UTC Pre-commit check for e12ca69 has started.
⚪ 2024-07-26 16:09:59 UTC Check linux-x86_64-release-clang14 is running...
🔴 2024-07-26 16:10:14 UTC Build failed. see the logs.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 26, 2024 •
edited
Loading

⚪ 2024-07-26 16:06:35 UTC Pre-commit check for e12ca69 has started.
⚪ 2024-07-26 16:10:08 UTC Check linux-x86_64-relwithdebinfo is running...
🔴 2024-07-26 16:10:21 UTC Build failed. see the logs.

snaury
snaury reviewed Jul 26, 2024
View reviewed changes
ydb/library/yql/dq/actors/dq_events_ids.h Outdated
@@ -1,14 +1,15 @@
#pragma once

#include <ydb/core/base/events.h>
Copy link
Member

@snaury snaury Jul 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не должно быть зависимостей из library в core. Скорее всего именно по этой причине ниже номера дублировали. Edit: впрочем похоже это правило уже очень давно нарушилось. :-/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну мне кажется тут явно какая-то проблема есть: точка правды очевидно там где енум EEventSpaceKikimr и все остальные должны на него ссылаться

Если сейчас какие-то другие правила противоречат этому, то надо как мне кажется распутывать этот клубок

Но я предлагаю просто здесь изменения откатить и ассерт мне добавить что значения совпадают

@maximyurchuk maximyurchuk removed request for a team July 27, 2024 21:41
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 27, 2024 •
edited
Loading

⚪ 2024-07-27 21:44:28 UTC Pre-commit check for a082f7a has started.
⚪ 2024-07-27 21:47:41 UTC Check linux-x86_64-release-clang14 is running...
🟢 2024-07-27 22:27:17 UTC Build successful.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 27, 2024 •
edited
Loading

⚪ 2024-07-27 21:44:33 UTC Pre-commit check for a082f7a has started.
⚪ 2024-07-27 21:47:49 UTC Check linux-x86_64-relwithdebinfo is running...
🟡 2024-07-28 00:07:05 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
39754 34464 0 1 5274 15

⚪ 2024-07-28 00:11:03 UTC Failed tests rerun (try 2) linux-x86_64-relwithdebinfo is running...
🟢 2024-07-28 00:19:17 UTC Tests successful.

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
16 (only retried tests) 6 0 0 1 9

🟢 2024-07-28 00:22:36 UTC Build successful.
🟢 2024-07-28 00:23:18 UTC ydbd size 8.1 GiB changed* by -16 Bytes, which is <= 0 Bytes vs main: OK

ydbd size dash main: a6db73d merge: a082f7a diff diff %
ydbd size 8 665 852 344 Bytes 8 665 852 328 Bytes -16 Bytes -0.000%
ydbd stripped size 471 473 344 Bytes 471 473 344 Bytes 0 Bytes 0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 27, 2024 •
edited
Loading

⚪ 2024-07-27 21:45:04 UTC Pre-commit check for a082f7a has started.
⚪ 2024-07-27 21:48:14 UTC Check linux-x86_64-release-asan is running...
🔴 2024-07-28 00:30:07 UTC Some tests failed, follow the links below.

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
11082 10493 0 85 324 180

🟢 2024-07-28 00:31:10 UTC Build successful.
🟢 2024-07-28 00:32:15 UTC ydbd size 5.2 GiB changed* by -96 Bytes, which is <= 0 Bytes vs main: OK

ydbd size dash main: a6db73d merge: a082f7a diff diff %
ydbd size 5 564 152 824 Bytes 5 564 152 728 Bytes -96 Bytes -0.000%
ydbd stripped size 1 198 142 800 Bytes 1 198 142 736 Bytes -64 Bytes -0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

@maximyurchuk maximyurchuk merged commit f0a033e into ydb-platform:main Jul 29, 2024
10 of 12 checks passed
maximyurchuk added a commit to maximyurchuk/ydb that referenced this pull request Jul 29, 2024
@maximyurchuk
Freeze event spaces ids (ydb-platform#7151)
225f44a
@maximyurchuk maximyurchuk mentioned this pull request Jul 29, 2024
Merged
@maximyurchuk maximyurchuk mentioned this pull request Jul 29, 2024
Merged
@GrigoriyPA GrigoriyPA mentioned this pull request Jul 30, 2024
Merged
MrLolthe1st pushed a commit to MrLolthe1st/ydb that referenced this pull request Aug 12, 2024
@maximyurchuk @MrLolthe1st
Freeze event spaces ids (ydb-platform#7151)
775bd2b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snaury snaury snaury approved these changes

Assignees
No one assigned
Labels
not-for-changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maximyurchuk @snaury